Skip to content

18n-2026-neo#299

Merged
katherineqian merged 4 commits intoberkmancenter:stagingfrom
audreyt:i18n-2026-neo
Feb 26, 2026
Merged

18n-2026-neo#299
katherineqian merged 4 commits intoberkmancenter:stagingfrom
audreyt:i18n-2026-neo

Conversation

@bestian
Copy link
Contributor

@bestian bestian commented Feb 7, 2026

What is in this PR?

This PR initializes the i18n work by branching from the current staging branch into i18n-2026-neo.

The main goal of this PR is not to complete the full translation, but to clearly signal that the i18n work has started, so upstream maintainers can review progress early and merge incrementally if desired.

This PR focuses on translating the first small chunk (14 strings) as a starting point.


Changes in the codebase

  • Introduced initial multi-language translations on top of the latest staging
  • Translated the first set of 14 strings to establish structure and workflow
  • Ensured all changes are aligned with existing i18n patterns
  • Kept automated tests updated alongside each change so that all tests pass

Changes outside the codebase

  • None
    (No external services, infrastructure, or database changes are introduced in this PR.)

Testing this PR

  • All existing automated tests pass
  • No special testing steps are required beyond the current CI pipeline
  • Each future translation update will continue to keep tests in sync

Additional information

  • This PR is intentionally small and conservative
  • The branch will be continuously updated with further translations
  • Upstream is welcome to review, comment, or merge at any point during the process
  • The goal is to keep the i18n work transparent, reviewable, and low-risk

the first little chunk.
@epenn
Copy link
Contributor

epenn commented Feb 10, 2026

Thank you so much Bestian! I appreciate the iterative approach you're taking as well. I'll make sure I review this week!

@epenn epenn self-requested a review February 10, 2026 22:08
@epenn
Copy link
Contributor

epenn commented Feb 17, 2026

Update: Will review this later today!

Copy link
Contributor

@epenn epenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

  • Automated tests are still passing.
  • No obvious visual issues as a result of i18n-ifying those strings.
  • And as mentioned earlier I appreciate the iterative approach you're taking as well.

I'm going to temporarily hold off on merging this while I'm confirming that a fix I made in our staging environment held up, but this looks good!

@bestian
Copy link
Contributor Author

bestian commented Feb 19, 2026

@epenn Thank you for review. I'll keep working on it.

LGTM!

  • Automated tests are still passing.
  • No obvious visual issues as a result of i18n-ifying those strings.
  • And as mentioned earlier I appreciate the iterative approach you're taking as well.

I'm going to temporarily hold off on merging this while I'm confirming that a fix I made in our staging environment held up, but this looks good!

@epenn Thank you for the review and feedback! I’ll continue refining it.

@bestian
Copy link
Contributor Author

bestian commented Feb 22, 2026

@epenn

PR Follow-up: Localize 20 User-Facing Static Strings

Summary

This follow-up localizes 20 previously hardcoded English strings across the app and updates tests so they no longer depend on exact English text. All user-facing copy now goes through the l10n module and is available in English, Spanish, Simplified Chinese, and Traditional Chinese (Taiwan).

Changes

1. New l10n keys (4 ARB files)

Added 20 keys to lib/l10n/app_en.arb, app_es.arb, app_zh.arb, and app_zh_Hant_TW.arb:

  • Validation: titleIsRequired, messageIsRequired, videoUrlIsRequired, imageUrlIsRequired, pleaseAddSomeAnswers, headlineIsRequired, headlineCannotBeEmpty, messageCannotBeEmpty, urlIsNotValid
  • Success / status: agendaItemWasSaved, agendaItemWasDeleted, postHasBeenCreated, postHasBeenUpdated, commentWasDeleted
  • UI labels: templateSettings, eventSettings, preEvent, postEvent, updateAction, postAction

(Existing postWasDeleted and wordCloudPromptRequired are used where applicable.)

2. Source code updates (5 files)

  • agenda_item_presenter.dart — Validation messages and success toasts use appLocalizationService.getLocalization() (title, message, video/image URL, poll answers, word cloud prompt, headline; agenda item saved/deleted).
  • event_settings_presenter.dartgetTitle() returns templateSettings / eventSettings from l10n.
  • manipulate_discussion_thread_presenter.dartgetPositiveButtonText() uses updateAction / postAction; success toasts use postHasBeenCreated / postHasBeenUpdated. Added import 'package:client/services.dart'.
  • discussion_thread_presenter.dart — Delete toasts use postWasDeleted and commentWasDeleted. Added import 'package:client/services.dart'.
  • pre_post_card_widget_presenter.dart — Validation uses headlineCannotBeEmpty, messageCannotBeEmpty, urlIsNotValid; getTitle() uses preEvent / postEvent.

3. Test updates (6 files)

  • Assertions: Replaced exact string checks (e.g. expect(result, 'Title is required')) with non-empty checks (e.g. expect(result, isNotNull); expect(result!.isNotEmpty, isTrue)) or argThat(isNotEmpty) in verify() so tests are locale-agnostic.
  • Setup: Where presenters use AppLocalizationService, tests now register it in GetIt and load English localizations in setUpAll, and call GetIt.instance.reset() in tearDownAll.

Updated test files:

  • agenda_item_helper_test.dartareRequiredFieldsInput expectations.
  • pre_post_card_widget_presenter_test.dartvalidateHeadline, validateMessage, validateUrl, getTitle; added l10n setUpAll/tearDownAll.
  • manipulate_discussion_thread_presenter_test.dartgetPositiveButtonText, showMessage verifies; added l10n setup.
  • discussion_thread_presenter_test.dartdeleteThread / deleteComment showMessage verifies; added l10n setup.
  • agenda_item_presenter_test.dart — Added l10n setup so deleteAgendaItem (and any other tests using localization) works.
  • pre_post_card_widget_test.dart (widget test) — setUpAll now loads and sets English localizations so the widget can resolve l10n strings.

Result

  • flutter gen-l10n — Runs successfully; new keys are generated.
  • flutter test --platform chrome542 tests passed, 11 skipped, 0 failures (previously failing tests for event settings, agenda item, discussion thread, and pre/post card widget now pass).

Notes

  • No new dependencies; uses existing flutter_gen l10n and AppLocalizationService with GetIt.
  • Tests that only need to ensure “a message was shown” use isNotEmpty (or argThat(isNotEmpty)) instead of matching a specific translation.

Documents commands, architecture, and conventions to help Claude Code
work effectively in this repository without needing to re-discover
them each session.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@katherineqian
Copy link
Collaborator

katherineqian commented Feb 26, 2026

Hi @bestian, this all looks good to me. Just one note regarding CLAUDE.md -- I appreciate the addition, but since this isn't related directly to the i18n improvements, it would be most helpful to remove it for now so that it doesn't get merged into staging alongside the code changes. We can always make a new issue to add an official CLAUDE.md to the main codebase if you feel it would be helpful for the project, and the team can chime in with their thoughts too.

Hopefully this is okay with you -- to make things easier and save you some time, I just went ahead and reverted the CLAUDE.md commit instead of asking you to do it, and will now approve for merging!

@katherineqian katherineqian merged commit 55024b2 into berkmancenter:staging Feb 26, 2026
1 check passed
@bestian
Copy link
Contributor Author

bestian commented Feb 26, 2026

@katherineqian Thank you for the review and merge!

I’ve just created a PR to add Claude Code support. When you have a chance, I’d really appreciate it if you could take a look:

#314

katherineqian added a commit that referenced this pull request Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants